New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Websocket #711
Websocket #711
Conversation
POST to /@connections/{connectionId} to send message to client at connectionId.
Here's the new way: const endpoint=event.requestContext.domainName+'/'+event.requestContext.stage; const apiVersion='2018-11-29'; let API=null; if (!process.env.IS_OFFLINE) { API = require('aws-sdk'); require('aws-sdk/clients/apigatewaymanagementapi'); } else { API = require('serverless-offline').AWS; } return new API.ApiGatewayManagementApi({ apiVersion, endpoint });
1. Action of routes. 2. Listening on WS message sending via REST API.
Doesn't seem to be needed anymore as a workaround in aws-sdk.
Use plain vanilla AWS SDK: let endpoint=event.apiGatewayUrl; if (!endpoint) endpoint = event.requestContext.domainName+'/'+event.requestContext.stage; const apiVersion='2018-11-29'; return new AWS.ApiGatewayManagementApi({ apiVersion, endpoint });
… into websocket-support
In the form of $request.body.x.y.z
… into websocket-support
@computerpunc what do you mean by this? is this new, or was added/changed after your initial PR? |
close connection on error in $connect action handler
I was just looking over the Thinking about it further I'm leaning towards to remove @computerpunc Do you see any advantages over using That being said, I don't wanna hold up the PR, we can still change it later down the road. @dherault I'd say let's pull everything into master and release the kraken! I'd even go v6.0 - not necessarily as a breaking change -- although at least it signifies that something potentially could break --, but a major plugin addition! |
@computerpunc one more thing from above, it might have gotten buried: I couldn't find any information on |
d9a7ca8
to
c3fcc4c
Compare
I didn’t add this (at least not intentionally) to the WS flow. I think it got in when splitting index into WS and HTTP. |
In my advanced branch with authorizer support, I already use a regular JS object for mapping with the key bring the connection ID and taken from the WebSocket client handshake key. In any case, we still need a map to go from connection ID to ws when we want to send messages back to clients.
In that branch, I already use a modified version of the plugin to overcome its limitations. |
I don't think so. I believe this is (now) in |
true. but having the 'id' as the key in the map would prevent from (in that case needless) traversing.
ok.
when you look at the current implementation and visually switch the |
removed apiGatewayUrl: c0bea4c, still need to fix the docs and manual tests. |
Sorry about that. It was rather in #725, though. Full disclosure: I could not get any test (neither those nor the existing ones) to pass on AWS so I guessed I had something missing in my environment, but I just copy-pasted the tests right above for the POST behaviour and changed the expected results. I also felt confident that the tests were all passing in Travis... If anything breaks, I would guess that the sigV4 URL is malformed or something like that (it does not need to be signed locally). It's probably because I forgot to remove the body. I will give it a go again this weekend (sorry, absolutely no time before that) but I'm really confident that it does work! But if one of you has a working environment and wants to give it a go, you can try this: frsechet@9cdad89 |
Yes, that solves it. Removing body and Content-Type header solves it. |
There's a small issue: |
I changed it to that, because I think it would be confusing (and annoying) behavior. Although I thought @dherault changed it back, not sure. imagine the following scenario: user uses default ports, 3000 and +1, now, for whatever reason gets a collision on 3000, and changes the settings for that. (which could involve several things: ngrok, nginx reverse proxy, and what not). now, all the sudden the websocket port also changes. |
Here you go: #729 |
fix: don't add body and content-type headers to sigv4
It's now Ok, let's merge shall we? |
@dherault done!! 😃 [gonna remove the remaining |
I'll wait for your commit and then publish the release |
@dherault alrighty, all done! the manual tests probably don't work anymore (as of this moment), but we can fix them with either with what you mentioned env.IS_OFFLINE ? localhost : http://aws .... or env options. it shouldn't hold back the release. it's just if anyone would look at the manual tests, the event property's existence doesn't cause any confusion. |
v5.6.0 🎉 |
includes #674 and some touchups.
To do: